Skip to content

[MINOR][CONNECT][PYTHON] Fix stale running-task count in progress bar - #57545

Open
fallintoplace wants to merge 1 commit into
apache:masterfrom
fallintoplace:minor-connect-progress-running-count
Open

[MINOR][CONNECT][PYTHON] Fix stale running-task count in progress bar#57545
fallintoplace wants to merge 1 commit into
apache:masterfrom
fallintoplace:minor-connect-progress-running-count

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Update the running-task and stage state before rendering the Spark Connect progress bar. Add a regression assertion for the displayed running-task count.

Why are the changes needed?

Progress.update_ticks() previously called output() before storing the latest inflight_tasks value. Each progress message therefore displayed the running-task count from the previous update.

For example, the first update with 10 running tasks displayed 0 Tasks running. This change ensures that the rendered message uses values from the same progress update.

Does this PR introduce any user-facing change?

Yes. The Spark Connect progress bar now displays the current running-task count instead of the count from the previous update.

How was this patch tested?

Added a regression assertion to ProgressBarTest.test_simple_progress.

PYTHONPATH=python python -m unittest \
  pyspark.sql.tests.connect.shell.test_progress.ProgressBarTest

All five tests passed. Ruff lint and formatting checks also passed.

Was this patch authored or co-authored using generative AI tooling?

No

@HyukjinKwon

Copy link
Copy Markdown
Member

Do you happen to know which change caused this regerssion?

@uros-b

uros-b commented Jul 26, 2026

Copy link
Copy Markdown
Member

Thank you @fallintoplace!

@fallintoplace

Copy link
Copy Markdown
Contributor Author

@HyukjinKwon I traced it back to #45150 (f6999df, [SPARK-47081][CONNECT] Support Query Execution Progress), committed on April 4, 2024. The original Progress.update_ticks() implementation already called output() before assigning self._running. The follow-up in #45907 kept the same ordering, so this appears to have been present since the Connect progress bar was introduced rather than caused by a later change.

@HyukjinKwon

Copy link
Copy Markdown
Member

Let's actually file a JIRA

@fallintoplace

Copy link
Copy Markdown
Contributor Author

Should I do this or are you on it? I can file a JIRA ticket in 20 hours. @HyukjinKwon

@fallintoplace

Copy link
Copy Markdown
Contributor Author

I don't have an account though, maybe someone can do this. I will need to wait for access.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants